-
Notifications
You must be signed in to change notification settings - Fork 868
feat: adds cluster-attributes to start cli command #7494
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: adds cluster-attributes to start cli command #7494
Conversation
Signed-off-by: David Porter <david.porter@uber.com>
| }, | ||
| &cli.StringFlag{ | ||
| Name: FlagClusterAttributeScope, | ||
| Usage: "Optional cluster attribute to specify how to select the active cluster. Examples might be 'region' or 'location'", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: As the command will fail if one of these two isn't specified I'd recommend adding something like
... Required if ClusterAttributeName is specified.
| }, | ||
| &cli.StringFlag{ | ||
| Name: FlagClusterAttributeName, | ||
| Usage: "Optional cluster attribute to be set for the workflow, used to determine, in active-active domains. This specifies which attribute to tie the workflow to, for example, if the scope is 'region' and the name is 'Lisbon' or 'San Francisco'", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: As the command will fail if one of these two isn't specified I'd recommend adding something like ... Required if ClusterAttributeScope is specified.
NIT: I think rephrasing to remove some commas is possible, e.g:
Optional cluster attribute name, paired with a cluster attribute scope, to specify how to select the active cluster. This specifies which attribute to tie the workflow to, for example, if the scope is 'region' and the name is 'Lisbon' or 'San Francisco'
(This doesn't actually reduce the number of commas but I think is more clear 😅 ).
ae38f0d
into
cadence-workflow:master
<!-- Describe what has changed in this PR --> **What changed?** A follow on to #7494 since the earlier pr autoclosed <!-- Tell your future self why have you made these changes --> **Why?** <!-- How have you verified this change? Tested locally? Added a unit test? Checked in staging env? --> **How did you test it?** <!-- Assuming the worst case, what can be broken when deploying this change to production? --> **Potential risks** <!-- Is it notable for release? e.g. schema updates, configuration or data migration required? If so, please mention it, and also update CHANGELOG.md --> **Release notes** <!-- Is there any documentation updates should be made for config, https://cadenceworkflow.io/docs/operation-guide/setup/ ? If so, please open an PR in https://github.com/cadence-workflow/cadence-docs --> **Documentation Changes** Signed-off-by: David Porter <david.porter@uber.com>
What changed?
Adds the ability to specify cluster attribute and cluster scope in the workflow start command. The goal is for customers who want to start workflows in multiple regions using the active-active feature, to be able to do so with manual cli invocations.
Why?
How did you test it?
tested locally for the start call, seems to work as expected.
Potential risks
Release notes
Documentation Changes